-
Notifications
You must be signed in to change notification settings - Fork 879
Add loading state and toast notifications to PingTab #671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add loading state and toast notifications to PingTab #671
Conversation
- Implement toast notifications for ping status (loading, success, error) - Add Bell icon to ping button for better UX
Hi @olishiz , thanks for the PR. Were you table to test this with a failed ping as well? |
Yes, I tested it locally as it seems when I shut down the server, it will show the error message. ![]() ![]() I believed the error will be handled properly and it will show on the toast message to the user |
@olishiz Thanks for this! I agree that the toast response is good and that the bell on the button is good. I have a suggestion / request. It can be done on another PR, but if we're monkeying with the ping button we might as well do it here if you're up for it. I have always felt that the
So I propose that it be refactor/moved to the Sidebar as a full-width button, vertically between those buttons and the connection state indicator: ![]() ![]() It should be hidden when @olaservo I think we talked in passing about this once before, does this sound reasonable? |
@cliffhall yes I think that makes sense. There's a lot more tabs now that actually deserve a tab, but I don't think ping needs it's own tab in the UI. |
Motivation and Context
How Has This Been Tested?
Production
Breaking Changes
No, only UI changes
Types of changes
Checklist
Additional context
Image as seen here:
